[libvirt] [PATCH 5/3] storage: always probe type with buffer
Eric Blake
eblake at redhat.com
Tue Nov 5 21:32:20 UTC 2013
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 at redhat.com>
---
This is the default 'git diff' output. The 'git diff --patience'
output is slightly longer, but might be slightly easier to read.
src/libvirt_private.syms | 2 +-
src/storage/storage_backend_fs.c | 53 +++++++++++++++++---------
src/util/virstoragefile.c | 81 +++++++++++++---------------------------
src/util/virstoragefile.h | 12 +++++-
4 files changed, 72 insertions(+), 76 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c1189d7..76016ca 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1731,7 +1731,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 58fb7c1..24579b0 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,26 @@ 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);
+ virStorageFileFreeMetadata(meta);
return ret;
}
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index c1faf41..e45236f 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);
@@ -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++) {
@@ -882,36 +877,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) {
@@ -919,44 +919,15 @@ virStorageFileProbeFormatFromFD(const char *path, int fd)
goto cleanup;
}
- if ((len = virFileReadHeaderFD(fd, len, &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;
@@ -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;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index f3d9832..7bd2fe0 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.8.3.1
More information about the libvir-list
mailing list