storage: always probe type with buffer

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>

src/libvirt_private.syms

@@ -1731,7 +1731,7 @@ virStorageFileIsClusterFS;
 virStorageFileIsSharedFS;
 virStorageFileIsSharedFSType;
 virStorageFileProbeFormat;
-virStorageFileProbeFormatFromFD;
+virStorageFileProbeFormatFromBuf;
 virStorageFileResize;
 
 

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) {
+    if (S_ISDIR(sb.st_mode)) {
-        ret = -1;
+        target->format = VIR_STORAGE_FILE_DIR;
-        goto error;
+    } 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 = virStorageFileProbeFormatFromBuf(target->path,
-                                                 target->format))) {
+                                                          header, len);
-        ret = -1;
+        if (target->format < 0) {
-        goto error;
+            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;
+    if (meta) {
-    meta->features = NULL;
+        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);
+    goto cleanup;
-
-    return ret;
 
 error:
     VIR_FORCE_CLOSE(fd);
 
 cleanup:
     virStorageFileFreeMetadata(meta);
+    VIR_FREE(header);
     return ret;
 
 }

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)
+    if (len > VIR_STORAGE_MAX_HEADER)
-        len = STORAGE_MAX_HEAD;
+        len = VIR_STORAGE_MAX_HEADER;
     memcpy(desc, buf + 0x200, len);
     desc[len] = '\0';
     start = strstr(desc, prefix);
@@ -682,7 +677,7 @@ virBackingStoreIsFile(const char *backing)
     return true;
 }
 
-static int
+int
 virStorageFileProbeFormatFromBuf(const char *path,
                                  char *buf,
                                  size_t buflen)
@@ -690,7 +685,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++) {
@@ -881,57 +876,6 @@ cleanup:
 }
 
 
-/**
- * virStorageFileProbeFormatFromFD:
- *
- * Probe for the format of 'fd' (which is an open file descriptor
- * pointing to '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
- * format at will.
- *
- * Best option: Don't use this function
- */
-int
-virStorageFileProbeFormatFromFD(const char *path, int fd)
-{
-    char *head = NULL;
-    ssize_t len = STORAGE_MAX_HEAD;
-    int ret = -1;
-    struct stat sb;
-
-    if (fstat(fd, &sb) < 0) {
-        virReportSystemError(errno,
-                             _("cannot stat file '%s'"),
-                             path);
-        return -1;
-    }
-
-    /* No header to probe for directories */
-    if (S_ISDIR(sb.st_mode)) {
-        return VIR_STORAGE_FILE_DIR;
-    }
-
-    if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
-        virReportSystemError(errno, _("cannot set to start of '%s'"), path);
-        goto cleanup;
-    }
-
-    if ((len = virFileReadHeaderFD(fd, len, &head)) < 0) {
-        virReportSystemError(errno, _("cannot read header '%s'"), path);
-        goto cleanup;
-    }
-
-    ret = virStorageFileProbeFormatFromBuf(path, head, len);
-
-cleanup:
-    VIR_FREE(head);
-    return ret;
-}
-
-
 /**
  * virStorageFileProbeFormat:
  *
@@ -948,15 +892,42 @@ cleanup:
 int
 virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)
 {
-    int fd, ret;
+    int fd;
+    int ret = -1;
+    struct stat sb;
+    ssize_t len = VIR_STORAGE_MAX_HEADER;
+    char *header = NULL;
 
     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);
+    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)) {
+        ret = VIR_STORAGE_FILE_DIR;
+        goto cleanup;
+    }
+
+    if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
+        virReportSystemError(errno, _("cannot set to start of '%s'"), path);
+        goto cleanup;
+    }
+
+    if ((len = virFileReadHeaderFD(fd, len, &header)) < 0) {
+        virReportSystemError(errno, _("cannot read header '%s'"), path);
+        goto cleanup;
+    }
+
+    ret = virStorageFileProbeFormatFromBuf(path, header, len);
+
+cleanup:
+    VIR_FREE(header);
     VIR_FORCE_CLOSE(fd);
 
     return ret;
@@ -1003,7 +974,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;
 

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 virStorageFileProbeFormatFromBuf(const char *path, char *buf,
-                                    int fd);
+                                     size_t buflen);
 
 virStorageFileMetadataPtr virStorageFileGetMetadata(const char *path,
                                                     int format,