From 6729ce720b3274d576524b14939017c8dc7a00bb Mon Sep 17 00:00:00 2001 Message-Id: <6729ce720b3274d576524b14939017c8dc7a00bb@dist-git> From: Eric Blake 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 (cherry picked from commit 348b4e254bea98c83107887c0cf64c6572063d64) Signed-off-by: Jiri Denemark --- 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