[libvirt] [PATCH 06/18] storage: util: Clean up arguments of virStorageFileGetMetadataInternal
Peter Krempa
pkrempa at redhat.com
Sun Apr 20 22:13:10 UTC 2014
Avoid passing lot of arguments into guts of metadata retrieval to fill
the actual structure. Temporarily fill the structure before passing it
down to the actual metadata extractor.
This will later help the inversion of the steps taken to extract the
metadata so that this function can be fully converted to
virStorageSource as the data struct.
---
src/util/virstoragefile.c | 164 +++++++++++++++++++++++-----------------------
1 file changed, 81 insertions(+), 83 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index a005e00..e37be06 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -788,81 +788,68 @@ qcow2GetFeatures(virBitmapPtr *features,
}
-/* Given a header in BUF with length LEN, as parsed from the file with
- * user-provided name PATH and opened from CANONPATH, and where any
- * relative backing file will be opened from DIRECTORY, and assuming
- * it has the given FORMAT, populate the newly-allocated META with
- * information about the file and its backing store. */
+/* Given a header in BUF with length LEN, as parsed from the storage file
+ * assuming it has the given FORMAT, populate information into META
+ * with information about the file and its backing store. Return format
+ * of the backing store as BACKING_FORMAT. PATH and FORMAT have to be
+ * pre-populated in META*/
static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(7)
-ATTRIBUTE_NONNULL(8)
-virStorageFileGetMetadataInternal(const char *path,
- const char *canonPath,
- const char *directory,
+ATTRIBUTE_NONNULL(4)
+virStorageFileGetMetadataInternal(virStorageFileMetadataPtr meta,
char *buf,
size_t len,
- int format,
- virStorageFileMetadataPtr meta,
int *backingFormat)
{
int ret = -1;
- VIR_DEBUG("path=%s, canonPath=%s, dir=%s, buf=%p, len=%zu, format=%d",
- path, canonPath, directory, buf, len, format);
+ VIR_DEBUG("path=%s, buf=%p, len=%zu, meta->format=%d",
+ meta->path, buf, len, meta->format);
- if (VIR_STRDUP(meta->path, path) < 0)
- goto cleanup;
- if (VIR_STRDUP(meta->canonPath, canonPath) < 0)
- goto cleanup;
- if (VIR_STRDUP(meta->relDir, directory) < 0)
- goto cleanup;
+ if (meta->format == VIR_STORAGE_FILE_AUTO)
+ meta->format = virStorageFileProbeFormatFromBuf(meta->path, buf, len);
- 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);
+ if (meta->format <= VIR_STORAGE_FILE_NONE ||
+ meta->format >= VIR_STORAGE_FILE_LAST) {
+ virReportSystemError(EINVAL, _("unknown storage file meta->format %d"),
+ meta->format);
goto cleanup;
}
- meta->format = format;
/* XXX we should consider moving virStorageBackendUpdateVolInfo
* code into this method, for non-magic files
*/
- if (!fileTypeInfo[format].magic)
+ if (!fileTypeInfo[meta->format].magic)
goto done;
/* Optionally extract capacity from file */
- if (fileTypeInfo[format].sizeOffset != -1) {
- if ((fileTypeInfo[format].sizeOffset + 8) > len)
+ if (fileTypeInfo[meta->format].sizeOffset != -1) {
+ if ((fileTypeInfo[meta->format].sizeOffset + 8) > len)
goto done;
- if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN)
+ if (fileTypeInfo[meta->format].endian == LV_LITTLE_ENDIAN)
meta->capacity = virReadBufInt64LE(buf +
- fileTypeInfo[format].sizeOffset);
+ fileTypeInfo[meta->format].sizeOffset);
else
meta->capacity = virReadBufInt64BE(buf +
- fileTypeInfo[format].sizeOffset);
+ fileTypeInfo[meta->format].sizeOffset);
/* Avoid unlikely, but theoretically possible overflow */
if (meta->capacity > (ULLONG_MAX /
- fileTypeInfo[format].sizeMultiplier))
+ fileTypeInfo[meta->format].sizeMultiplier))
goto done;
- meta->capacity *= fileTypeInfo[format].sizeMultiplier;
+ meta->capacity *= fileTypeInfo[meta->format].sizeMultiplier;
}
- if (fileTypeInfo[format].qcowCryptOffset != -1) {
+ if (fileTypeInfo[meta->format].qcowCryptOffset != -1) {
int crypt_format;
crypt_format = virReadBufInt32BE(buf +
- fileTypeInfo[format].qcowCryptOffset);
+ fileTypeInfo[meta->format].qcowCryptOffset);
if (crypt_format && VIR_ALLOC(meta->encryption) < 0)
goto cleanup;
}
- if (fileTypeInfo[format].getBackingStore != NULL) {
- int store = fileTypeInfo[format].getBackingStore(&meta->backingStoreRaw,
+ if (fileTypeInfo[meta->format].getBackingStore != NULL) {
+ int store = fileTypeInfo[meta->format].getBackingStore(&meta->backingStoreRaw,
backingFormat,
buf, len);
if (store == BACKING_STORE_INVALID)
@@ -872,11 +859,11 @@ virStorageFileGetMetadataInternal(const char *path,
goto cleanup;
}
- if (fileTypeInfo[format].getFeatures != NULL &&
- fileTypeInfo[format].getFeatures(&meta->features, format, buf, len) < 0)
+ if (fileTypeInfo[meta->format].getFeatures != NULL &&
+ fileTypeInfo[meta->format].getFeatures(&meta->features, meta->format, buf, len) < 0)
goto cleanup;
- if (format == VIR_STORAGE_FILE_QCOW2 && meta->features &&
+ if (meta->format == VIR_STORAGE_FILE_QCOW2 && meta->features &&
VIR_STRDUP(meta->compat, "1.1") < 0)
goto cleanup;
@@ -946,6 +933,34 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)
}
+static virStorageFileMetadataPtr
+virStorageFileMetadataNew(const char *path,
+ int format)
+{
+ virStorageFileMetadataPtr ret = NULL;
+
+ if (VIR_ALLOC(ret) < 0)
+ return NULL;
+
+ ret->format = format;
+ ret->type = VIR_STORAGE_TYPE_FILE;
+
+ if (VIR_STRDUP(ret->path, path) < 0)
+ goto error;
+
+ if (!(ret->canonPath = canonicalize_file_name(path))) {
+ virReportSystemError(errno, _("unable to resolve '%s'"), path);
+ goto error;
+ }
+
+ return ret;
+
+ error:
+ virStorageFileFreeMetadata(ret);
+ return NULL;
+}
+
+
/**
* virStorageFileGetMetadataFromBuf:
* @path: name of file, for error messages
@@ -979,19 +994,11 @@ virStorageFileGetMetadataFromBuf(const char *path,
int *backingFormat)
{
virStorageFileMetadataPtr ret = NULL;
- char *canonPath;
- if (!(canonPath = canonicalize_file_name(path)) &&
- VIR_STRDUP(canonPath, path) < 0) {
- virReportSystemError(errno, _("unable to resolve '%s'"), path);
+ if (!(ret = virStorageFileMetadataNew(path, format)))
return NULL;
- }
-
- if (VIR_ALLOC(ret) < 0)
- goto cleanup;
- if (virStorageFileGetMetadataInternal(path, canonPath, ".", buf, len,
- format, ret,
+ if (virStorageFileGetMetadataInternal(ret, buf, len,
backingFormat) < 0) {
virStorageFileFreeMetadata(ret);
ret = NULL;
@@ -1002,20 +1009,14 @@ virStorageFileGetMetadataFromBuf(const char *path,
ret = NULL;
}
- cleanup:
- VIR_FREE(canonPath);
return ret;
}
/* Internal version that also supports a containing directory name. */
static int
-virStorageFileGetMetadataFromFDInternal(const char *path,
- const char *canonPath,
- const char *directory,
+virStorageFileGetMetadataFromFDInternal(virStorageFileMetadataPtr meta,
int fd,
- int format,
- virStorageFileMetadataPtr meta,
int *backingFormat)
{
char *buf = NULL;
@@ -1026,11 +1027,13 @@ virStorageFileGetMetadataFromFDInternal(const char *path,
if (!backingFormat)
backingFormat = &dummy;
+
*backingFormat = VIR_STORAGE_FILE_NONE;
+
if (fstat(fd, &sb) < 0) {
virReportSystemError(errno,
_("cannot stat file '%s'"),
- path);
+ meta->path);
return -1;
}
@@ -1038,8 +1041,8 @@ virStorageFileGetMetadataFromFDInternal(const char *path,
/* No header to probe for directories, but also no backing
* file; therefore, no inclusion loop is possible, and we
* don't need canonName or relDir. */
- if (VIR_STRDUP(meta->path, path) < 0)
- goto cleanup;
+ VIR_FREE(meta->relDir);
+ VIR_FREE(meta->canonPath);
meta->type = VIR_STORAGE_TYPE_DIR;
meta->format = VIR_STORAGE_FILE_DIR;
ret = 0;
@@ -1047,18 +1050,16 @@ virStorageFileGetMetadataFromFDInternal(const char *path,
}
if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
- virReportSystemError(errno, _("cannot seek to start of '%s'"), path);
+ virReportSystemError(errno, _("cannot seek to start of '%s'"), meta->path);
goto cleanup;
}
if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) {
- virReportSystemError(errno, _("cannot read header '%s'"), path);
+ virReportSystemError(errno, _("cannot read header '%s'"), meta->path);
goto cleanup;
}
- ret = virStorageFileGetMetadataInternal(path, canonPath, directory,
- buf, len, format, meta,
- backingFormat);
+ ret = virStorageFileGetMetadataInternal(meta, buf, len, backingFormat);
if (ret == 0) {
if (S_ISREG(sb.st_mode))
@@ -1071,7 +1072,6 @@ virStorageFileGetMetadataFromFDInternal(const char *path,
return ret;
}
-
/**
* virStorageFileGetMetadataFromFD:
*
@@ -1091,23 +1091,16 @@ virStorageFileGetMetadataFromFD(const char *path,
int format)
{
virStorageFileMetadataPtr ret = NULL;
- char *canonPath;
- if (!(canonPath = canonicalize_file_name(path))) {
- virReportSystemError(errno, _("unable to resolve '%s'"), path);
- return NULL;
- }
- if (VIR_ALLOC(ret) < 0)
+ if (!(ret = virStorageFileMetadataNew(path, format)))
goto cleanup;
- if (virStorageFileGetMetadataFromFDInternal(path, canonPath, ".",
- fd, format, ret,
- NULL) < 0) {
+
+ if (virStorageFileGetMetadataFromFDInternal(ret, fd, NULL) < 0) {
virStorageFileFreeMetadata(ret);
ret = NULL;
}
cleanup:
- VIR_FREE(canonPath);
return ret;
}
@@ -1139,15 +1132,20 @@ virStorageFileGetMetadataRecurse(const char *path, const char *canonPath,
return -1;
if (virStorageIsFile(path)) {
+ if (VIR_STRDUP(meta->path, path) < 0)
+ return -1;
+ if (VIR_STRDUP(meta->canonPath, canonPath) < 0)
+ return -1;
+ if (VIR_STRDUP(meta->relDir, directory) < 0)
+ return -1;
+ meta->format = format;
+
if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) {
virReportSystemError(-fd, _("Failed to open file '%s'"), path);
return -1;
}
- ret = virStorageFileGetMetadataFromFDInternal(path, canonPath,
- directory,
- fd, format, meta,
- &backingFormat);
+ ret = virStorageFileGetMetadataFromFDInternal(meta, fd, &backingFormat);
if (VIR_CLOSE(fd) < 0)
VIR_WARN("could not close file %s", path);
--
1.9.1
More information about the libvir-list
mailing list