From 6729ce720b3274d576524b14939017c8dc7a00bb Mon Sep 17 00:00:00 2001

Message-Id: <6729ce720b3274d576524b14939017c8dc7a00bb@dist-git>

From: Eric Blake <eblake@redhat.com>

Date: Wed, 26 Feb 2014 14:54:35 +0100

Subject: [PATCH] storage: always probe type with buffer

https://bugzilla.redhat.com/show_bug.cgi?id=1032370

This gets rid of another stat() per volume, as well as cutting

bytes read in half, when populating the volumes of a directory

pool during a pool refresh.  Not to mention that it provides an

interface that can let gluster pools also probe file types.

* src/util/virstoragefile.h (virStorageFileProbeFormatFromFD):

Delete.

(virStorageFileProbeFormatFromBuf): New prototype.

(VIR_STORAGE_MAX_HEADER): New constant, based on...

* src/util/virstoragefile.c (STORAGE_MAX_HEAD): ...old name.

(vmdk4GetBackingStore, virStorageFileGetMetadataInternal)

(virStorageFileProbeFormat): Adjust clients.

(virStorageFileProbeFormatFromFD): Delete.

(virStorageFileProbeFormatFromBuf): Export.

* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):

Adjust client.

* src/libvirt_private.syms (virstoragefile.h): Adjust exports.

Signed-off-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit 348b4e254bea98c83107887c0cf64c6572063d64)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/libvirt_private.syms         |  2 +-

 src/storage/storage_backend_fs.c | 52 +++++++++++++++++---------

 src/util/virstoragefile.c        | 81 +++++++++++++---------------------------

 src/util/virstoragefile.h        | 12 +++++-

 4 files changed, 71 insertions(+), 76 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms

index 4d74d46..4d80130 100644

--- a/src/libvirt_private.syms

+++ b/src/libvirt_private.syms

@@ -1931,7 +1931,7 @@ virStorageFileIsClusterFS;

 virStorageFileIsSharedFS;

 virStorageFileIsSharedFSType;

 virStorageFileProbeFormat;

-virStorageFileProbeFormatFromFD;

+virStorageFileProbeFormatFromBuf;

 virStorageFileResize;

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c

index 5d68232..11bb39b 100644

--- a/src/storage/storage_backend_fs.c

+++ b/src/storage/storage_backend_fs.c

@@ -1,7 +1,7 @@

 /*

  * storage_backend_fs.c: storage backend for FS and directory handling

  *

- * Copyright (C) 2007-2012 Red Hat, Inc.

+ * Copyright (C) 2007-2013 Red Hat, Inc.

  * Copyright (C) 2007-2008 Daniel P. Berrange

  *

  * This library is free software; you can redistribute it and/or

@@ -71,6 +71,8 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,

     int ret = -1;

     virStorageFileMetadata *meta = NULL;

     struct stat sb;

+    char *header = NULL;

+    ssize_t len = VIR_STORAGE_MAX_HEADER;

     *backingStore = NULL;

     *backingStoreFormat = VIR_STORAGE_FILE_AUTO;

@@ -88,20 +90,33 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,

         goto error;

     }

-    if ((target->format = virStorageFileProbeFormatFromFD(target->path, fd)) < 0) {

-        ret = -1;

-        goto error;

-    }

+    if (S_ISDIR(sb.st_mode)) {

+        target->format = VIR_STORAGE_FILE_DIR;

+    } else {

+        if ((len = virFileReadHeaderFD(fd, len, &header)) < 0) {

+            virReportSystemError(errno, _("cannot read header '%s'"),

+                                 target->path);

+            goto error;

+        }

-    if (!(meta = virStorageFileGetMetadataFromFD(target->path, fd,

-                                                 target->format))) {

-        ret = -1;

-        goto error;

+        target->format = virStorageFileProbeFormatFromBuf(target->path,

+                                                          header, len);

+        if (target->format < 0) {

+            ret = -1;

+            goto error;

+        }

+

+        if (!(meta = virStorageFileGetMetadataFromBuf(target->path,

+                                                      header, len,

+                                                      target->format))) {

+            ret = -1;

+            goto error;

+        }

     }

     VIR_FORCE_CLOSE(fd);

-    if (meta->backingStore) {

+    if (meta && meta->backingStore) {

         *backingStore = meta->backingStore;

         meta->backingStore = NULL;

         if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO &&

@@ -126,10 +141,10 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,

         ret = 0;

     }

-    if (capacity && meta->capacity)

+    if (capacity && meta && meta->capacity)

         *capacity = meta->capacity;

-    if (encryption != NULL && meta->encrypted) {

+    if (encryption && meta && meta->encrypted) {

         if (VIR_ALLOC(*encryption) < 0)

             goto cleanup;

@@ -150,24 +165,25 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,

     }

     virBitmapFree(target->features);

-    target->features = meta->features;

-    meta->features = NULL;

+    if (meta) {

+        target->features = meta->features;

+        meta->features = NULL;

+    }

-    if (meta->compat) {

+    if (meta && meta->compat) {

         VIR_FREE(target->compat);

         target->compat = meta->compat;

         meta->compat = NULL;

     }

-    virStorageFileFreeMetadata(meta);

-

-    return ret;

+    goto cleanup;

 error:

     VIR_FORCE_CLOSE(fd);

 cleanup:

     virStorageFileFreeMetadata(meta);

+    VIR_FREE(header);

     return ret;

 }

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c

index 8b89db1..471287c 100644

--- a/src/util/virstoragefile.c

+++ b/src/util/virstoragefile.c

@@ -148,11 +148,6 @@ qedGetBackingStore(char **, int *, const char *, size_t);

 #define QED_F_BACKING_FILE 0x01

 #define QED_F_BACKING_FORMAT_NO_PROBE 0x04

-/* VMDK needs at least 20*512 B to find backing store,

- * ISO has 5 Byte magic on offset 32769,

- * other formats need less */

-#define STORAGE_MAX_HEAD 32769+5

-

 static struct FileTypeInfo const fileTypeInfo[] = {

     [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,

@@ -442,7 +437,7 @@ vmdk4GetBackingStore(char **res,

     size_t len;

     int ret = BACKING_STORE_ERROR;

-    if (VIR_ALLOC_N(desc, STORAGE_MAX_HEAD + 1) < 0)

+    if (VIR_ALLOC_N(desc, VIR_STORAGE_MAX_HEADER) < 0)

         goto cleanup;

     *res = NULL;

@@ -460,8 +455,8 @@ vmdk4GetBackingStore(char **res,

         goto cleanup;

     }

     len = buf_size - 0x200;

-    if (len > STORAGE_MAX_HEAD)

-        len = STORAGE_MAX_HEAD;

+    if (len > VIR_STORAGE_MAX_HEADER)

+        len = VIR_STORAGE_MAX_HEADER;

     memcpy(desc, buf + 0x200, len);

     desc[len] = '\0';

     start = strstr(desc, prefix);

@@ -677,7 +672,7 @@ virBackingStoreIsFile(const char *backing)

     return true;

 }

-static int

+int

 virStorageFileProbeFormatFromBuf(const char *path,

                                  char *buf,

                                  size_t buflen)

@@ -685,7 +680,7 @@ virStorageFileProbeFormatFromBuf(const char *path,

     int format = VIR_STORAGE_FILE_RAW;

     size_t i;

     int possibleFormat = VIR_STORAGE_FILE_RAW;

-    VIR_DEBUG("path=%s", path);

+    VIR_DEBUG("path=%s, buf=%p, buflen=%zu", path, buf, buflen);

     /* First check file magic */

     for (i = 0; i < VIR_STORAGE_FILE_LAST; i++) {

@@ -877,36 +872,41 @@ cleanup:

 /**

- * virStorageFileProbeFormatFromFD:

+ * virStorageFileProbeFormat:

  *

- * Probe for the format of 'fd' (which is an open file descriptor

- * pointing to 'path'), returning the detected disk format.

+ * Probe for the format of 'path', returning the detected

+ * disk format.

  *

  * Callers are advised never to trust the returned 'format'

  * unless it is listed as VIR_STORAGE_FILE_RAW, since a

- * malicious guest can turn a file into any other non-raw

+ * malicious guest can turn a raw file into any other non-raw

  * format at will.

  *

  * Best option: Don't use this function

  */

 int

-virStorageFileProbeFormatFromFD(const char *path, int fd)

+virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)

 {

-    char *head = NULL;

-    ssize_t len = STORAGE_MAX_HEAD;

+    int fd;

     int ret = -1;

     struct stat sb;

+    ssize_t len = VIR_STORAGE_MAX_HEADER;

+    char *header = NULL;

-    if (fstat(fd, &sb) < 0) {

-        virReportSystemError(errno,

-                             _("cannot stat file '%s'"),

-                             path);

+    if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) {

+        virReportSystemError(-fd, _("Failed to open file '%s'"), path);

         return -1;

     }

+    if (fstat(fd, &sb) < 0) {

+        virReportSystemError(errno, _("cannot stat file '%s'"), path);

+        goto cleanup;

+    }

+

     /* No header to probe for directories */

     if (S_ISDIR(sb.st_mode)) {

-        return VIR_STORAGE_FILE_DIR;

+        ret = VIR_STORAGE_FILE_DIR;

+        goto cleanup;

     }

     if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {

@@ -914,44 +914,15 @@ virStorageFileProbeFormatFromFD(const char *path, int fd)

         goto cleanup;

     }

-    if ((len = virFileReadHeaderFD(fd, len, (char **)&head)) < 0) {

+    if ((len = virFileReadHeaderFD(fd, len, &header)) < 0) {

         virReportSystemError(errno, _("cannot read header '%s'"), path);

         goto cleanup;

     }

-    ret = virStorageFileProbeFormatFromBuf(path, head, len);

+    ret = virStorageFileProbeFormatFromBuf(path, header, len);

 cleanup:

-    VIR_FREE(head);

-    return ret;

-}

-

-

-/**

- * virStorageFileProbeFormat:

- *

- * Probe for the format of 'path', returning the detected

- * disk format.

- *

- * Callers are advised never to trust the returned 'format'

- * unless it is listed as VIR_STORAGE_FILE_RAW, since a

- * malicious guest can turn a raw file into any other non-raw

- * format at will.

- *

- * Best option: Don't use this function

- */

-int

-virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)

-{

-    int fd, ret;

-

-    if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) {

-        virReportSystemError(-fd, _("Failed to open file '%s'"), path);

-        return -1;

-    }

-

-    ret = virStorageFileProbeFormatFromFD(path, fd);

-

+    VIR_FREE(header);

     VIR_FORCE_CLOSE(fd);

     return ret;

@@ -998,7 +969,7 @@ virStorageFileGetMetadataFromFDInternal(const char *path,

                                         int format)

 {

     char *buf = NULL;

-    ssize_t len = STORAGE_MAX_HEAD;

+    ssize_t len = VIR_STORAGE_MAX_HEADER;

     struct stat sb;

     virStorageFileMetadataPtr ret = NULL;

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h

index 5fc8832..e1ac91b 100644

--- a/src/util/virstoragefile.h

+++ b/src/util/virstoragefile.h

@@ -27,6 +27,14 @@

 # include "virbitmap.h"

 # include "virutil.h"

+/* Minimum header size required to probe all known formats with

+ * virStorageFileProbeFormat, or obtain metadata from a known format.

+ * Rounded to multiple of 512 (ISO has a 5-byte magic at offset

+ * 32769).  Some formats can be probed with fewer bytes.  Although

+ * some formats theoretically permit metadata that can rely on offsets

+ * beyond this size, in practice that doesn't matter.  */

+# define VIR_STORAGE_MAX_HEADER 0x8200

+

 enum virStorageFileFormat {

     VIR_STORAGE_FILE_AUTO_SAFE = -2,

     VIR_STORAGE_FILE_AUTO = -1,

@@ -80,8 +88,8 @@ struct _virStorageFileMetadata {

 # endif

 int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);

-int virStorageFileProbeFormatFromFD(const char *path,

-                                    int fd);

+int virStorageFileProbeFormatFromBuf(const char *path, char *buf,

+                                     size_t buflen);

 virStorageFileMetadataPtr virStorageFileGetMetadata(const char *path,

                                                     int format,

-- 

1.9.0