On Mon, Apr 28, 2014 at 09:53:54PM -0600, Eric Blake wrote: ACK, Martin

All callers of virStorageFileGetMetadataFromBuf were first calling virStorageFileProbeFormatFromBuf, to learn what format to pass in. But this function is already wired to do the exact same probe if the incoming format is VIR_STORAGE_FILE_AUTO, so it's simpler to just refactor the probing into the central function. * src/util/virstoragefile.h (virStorageFileGetMetadataFromBuf): Drop parameter. (virStorageFileProbeFormatFromBuf): Drop declaration. * src/util/virstoragefile.c (virStorageFileGetMetadataFromBuf): Do probe here instead of in callers. (virStorageFileProbeFormatFromBuf): Make static. * src/libvirt_private.syms (virstoragefile.h): Drop function. * src/storage/storage_backend_fs.c (virStorageBackendProbeTarget): Update caller. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshVol): Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/libvirt_private.syms | 1 - src/storage/storage_backend_fs.c | 8 -------- src/storage/storage_backend_gluster.c | 5 ----- src/util/virstoragefile.c | 17 +++++++---------- src/util/virstoragefile.h | 7 ++----- 5 files changed, 9 insertions(+), 29 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8bbe6e7..c95fa73 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1854,7 +1854,6 @@ virStorageFileGetSCSIKey; virStorageFileIsClusterFS; virStorageFileParseChainIndex; virStorageFileProbeFormat; -virStorageFileProbeFormatFromBuf; virStorageFileResize; virStorageIsFile; virStorageNetHostDefClear; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0f98853..f18fafe 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -97,16 +97,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, 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, backingStore, backingStoreFormat))) { ret = -1; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 5ab1e7e..20fb63e 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -293,12 +293,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, if ((len = virStorageBackendGlusterReadHeader(fd, name, len, &header)) < 0) goto cleanup; - if ((vol->target.format = virStorageFileProbeFormatFromBuf(name, - header, - len)) < 0) - goto cleanup; if (!(meta = virStorageFileGetMetadataFromBuf(name, header, len, - vol->target.format, &vol->backingStore.path, &vol->backingStore.format))) goto cleanup; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index dcce1ef..5dbb6f0 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -713,7 +713,8 @@ virStorageIsFile(const char *backing) return true; } -int + +static int virStorageFileProbeFormatFromBuf(const char *path, char *buf, size_t buflen) @@ -971,16 +972,13 @@ virStorageFileMetadataNew(const char *path, * @path: name of file, for error messages * @buf: header bytes from @path * @len: length of @buf - * @format: expected image format * @backing: output malloc'd name of backing image, if any * @backingFormat: format of @backing * - * 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 + * Extract metadata about the storage volume, including probing its + * format. Does not recurse. Callers are advised not to trust the + * learned format if a guest has ever used the volume when it was + * raw, since a malicious guest can turn a raw file into any * other non-raw format at will. * * If the returned @backingFormat is VIR_STORAGE_FILE_AUTO @@ -994,14 +992,13 @@ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path, char *buf, size_t len, - int format, char **backing, int *backingFormat) { virStorageSourcePtr ret = NULL; virStorageSourcePtr meta = NULL; - if (!(meta = virStorageFileMetadataNew(path, format))) + if (!(meta = virStorageFileMetadataNew(path, VIR_STORAGE_FILE_AUTO))) return NULL; if (virStorageFileGetMetadataInternal(meta, buf, len, diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 148776e..8592d13 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -260,8 +260,6 @@ struct _virStorageSource { # endif int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid); -int virStorageFileProbeFormatFromBuf(const char *path, char *buf, - size_t buflen); int virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, @@ -273,11 +271,10 @@ virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path, virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path, char *buf, size_t len, - int format, char **backing, int *backingFormat) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) - ATTRIBUTE_NONNULL(6); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(5); int virStorageFileChainGetBroken(virStorageSourcePtr chain, char **broken_file); -- 1.9.0