11:10 p.m.
Prior to this patch, we had the callchains:
external users
\-> virStorageFileGetMetadataFromFD
\-> virStorageFileGetMetadataFromBuf
virStorageFileGetMetadataRecurse
\-> virStorageFileGetMetadataFromFD
\-> virStorageFileGetMetadataFromBuf
However, a future patch wants to add an additional parameter to
the bottom of the chain, for use by virStorageFileGetMetadataRecurse,
without affecting existing external callers. Since there is only a
single caller of the internal function, we can repurpose it to fit
our needs, with this patch giving us:
external users
\-> virStorageFileGetMetadataFromFD
\-> virStorageFileGetMetadataInternal
virStorageFileGetMetadataRecurse /
\-> virStorageFileGetMetadataInternal
Note that no callers cared whether virStorageFileGetMetadataFromBuf
returned 0 (success) or 1 (able to read, but not able to decipher),
only whether there was failure (not able to read).
* src/util/virstoragefile.c (virStorageFileGetMetadataFromFD):
Move most of the guts...
(virStorageFileGetMetadataFromBuf): ...here, and rename...
(virStorageFileGetMetadataInternal): ...to this.
(virStorageFileGetMetadataRecurse): Use internal helper.
---
src/util/virstoragefile.c | 156 ++++++++++++++++++++++------------------------
1 file changed, 75 insertions(+), 81 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cfd424d..1a4557e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -708,28 +708,70 @@ cleanup:
}
-static int
-virStorageFileGetMetadataFromBuf(int format,
- const char *path,
- unsigned char *buf,
- size_t buflen,
- virStorageFileMetadata *meta)
+static virStorageFileMetadataPtr
+virStorageFileGetMetadataInternal(const char *path,
+ int fd,
+ int format)
{
- VIR_DEBUG("path=%s format=%d", path, format);
+ virStorageFileMetadata *meta = NULL;
+ unsigned char *buf = NULL;
+ ssize_t len = STORAGE_MAX_HEAD;
+ virStorageFileMetadata *ret = NULL;
+ struct stat sb;
+
+ VIR_DEBUG("path=%s, fd=%d, format=%d", path, fd, format);
+
+ if (VIR_ALLOC(meta) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ if (fstat(fd, &sb) < 0) {
+ virReportSystemError(errno,
+ _("cannot stat file '%s'"),
+ path);
+ goto cleanup;
+ }
+
+ /* No header to probe for directories, but also no backing file */
+ if (S_ISDIR(sb.st_mode))
+ return meta;
+
+ if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
+ virReportSystemError(errno, _("cannot seek to start of '%s'"),
path);
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC_N(buf, len) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if ((len = read(fd, buf, len)) < 0) {
+ virReportSystemError(errno, _("cannot read header '%s'"),
path);
+ goto cleanup;
+ }
+
+ if (format == VIR_STORAGE_FILE_AUTO)
+ format = virStorageFileProbeFormatFromBuf(path, buf, len);
+
+ if (format <= VIR_STORAGE_FILE_NONE ||
+ format >= VIR_STORAGE_FILE_LAST) {
+ virReportSystemError(EINVAL, _("unknown storage file format %d"),
+ format);
+ goto cleanup;
+ }
/* XXX we should consider moving virStorageBackendUpdateVolInfo
* code into this method, for non-magic files
*/
- if (format <= VIR_STORAGE_FILE_NONE ||
- format >= VIR_STORAGE_FILE_LAST ||
- !fileTypeInfo[format].magic) {
- return 0;
- }
+ if (!fileTypeInfo[format].magic)
+ goto done;
/* Optionally extract capacity from file */
if (fileTypeInfo[format].sizeOffset != -1) {
- if ((fileTypeInfo[format].sizeOffset + 8) > buflen)
- return 1;
+ if ((fileTypeInfo[format].sizeOffset + 8) > len)
+ goto done;
if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN)
meta->capacity = virRead64LE(buf +
@@ -740,7 +782,7 @@ virStorageFileGetMetadataFromBuf(int format,
/* Avoid unlikely, but theoretically possible overflow */
if (meta->capacity > (ULLONG_MAX /
fileTypeInfo[format].sizeMultiplier))
- return 1;
+ goto done;
meta->capacity *= fileTypeInfo[format].sizeMultiplier;
}
@@ -754,14 +796,14 @@ virStorageFileGetMetadataFromBuf(int format,
if (fileTypeInfo[format].getBackingStore != NULL) {
char *backing;
int backingFormat;
- int ret = fileTypeInfo[format].getBackingStore(&backing,
- &backingFormat,
- buf, buflen);
- if (ret == BACKING_STORE_INVALID)
- return 1;
+ int store = fileTypeInfo[format].getBackingStore(&backing,
+ &backingFormat,
+ buf, len);
+ if (store == BACKING_STORE_INVALID)
+ goto done;
- if (ret == BACKING_STORE_ERROR)
- return -1;
+ if (store == BACKING_STORE_ERROR)
+ goto cleanup;
meta->backingStoreIsFile = false;
if (backing != NULL) {
@@ -769,7 +811,7 @@ virStorageFileGetMetadataFromBuf(int format,
if (meta->backingStore == NULL) {
virReportOOMError();
VIR_FREE(backing);
- return -1;
+ goto cleanup;
}
if (virBackingStoreIsFile(backing)) {
meta->backingStoreIsFile = true;
@@ -794,7 +836,14 @@ virStorageFileGetMetadataFromBuf(int format,
}
}
- return 0;
+done:
+ ret = meta;
+ meta = NULL;
+
+cleanup:
+ virStorageFileFreeMetadata(meta);
+ VIR_FREE(buf);
+ return ret;
}
@@ -907,62 +956,7 @@ virStorageFileGetMetadataFromFD(const char *path,
int fd,
int format)
{
- virStorageFileMetadata *meta = NULL;
- unsigned char *head = NULL;
- ssize_t len = STORAGE_MAX_HEAD;
- virStorageFileMetadata *ret = NULL;
- struct stat sb;
-
- if (VIR_ALLOC(meta) < 0) {
- virReportOOMError();
- return NULL;
- }
-
- if (fstat(fd, &sb) < 0) {
- virReportSystemError(errno,
- _("cannot stat file '%s'"),
- path);
- goto cleanup;
- }
-
- /* No header to probe for directories, but also no backing file */
- if (S_ISDIR(sb.st_mode))
- return meta;
-
- if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
- virReportSystemError(errno, _("cannot seek to start of '%s'"),
path);
- goto cleanup;
- }
-
- if (VIR_ALLOC_N(head, len) < 0) {
- virReportOOMError();
- goto cleanup;
- }
-
- if ((len = read(fd, head, len)) < 0) {
- virReportSystemError(errno, _("cannot read header '%s'"),
path);
- goto cleanup;
- }
-
- if (format == VIR_STORAGE_FILE_AUTO)
- format = virStorageFileProbeFormatFromBuf(path, head, len);
-
- if (format <= VIR_STORAGE_FILE_NONE ||
- format >= VIR_STORAGE_FILE_LAST) {
- virReportSystemError(EINVAL, _("unknown storage file format %d"),
- format);
- goto cleanup;
- }
-
- if (virStorageFileGetMetadataFromBuf(format, path, head, len, meta) < 0)
- goto cleanup;
- ret = meta;
- meta = NULL;
-
-cleanup:
- virStorageFileFreeMetadata(meta);
- VIR_FREE(head);
- return ret;
+ return virStorageFileGetMetadataInternal(path, fd, format);
}
/* Recursive workhorse for virStorageFileGetMetadata. */
@@ -991,7 +985,7 @@ virStorageFileGetMetadataRecurse(const char *path, int format,
return NULL;
}
- ret = virStorageFileGetMetadataFromFD(path, fd, format);
+ ret = virStorageFileGetMetadataInternal(path, fd, format);
if (VIR_CLOSE(fd) < 0)
VIR_WARN("could not close file %s", path);
--
1.8.1