5:44 p.m.
On 04/20/2014 04:13 PM, Peter Krempa wrote:
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(-)
-/* 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*/
Cosmetic - I usually stick space before */
{
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);
The diff would be slightly smaller if you do:
int format = meta->format;
up front, then keep the original use of format everywhere else...
- 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);
...well, right here, you'd have to do 'format = meta->format =
virStorage...();', but the rest of the function has fewer changes. But
it doesn't matter to me; I'm fine keeping it as you wrote it.
+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))) {
Same complaint as in 3/18 - you should only call
canonicalize_file_name() if you KNOW that path should be interpreted as
a file name.
@@ -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;
- }
Minor merge conflicts with my suggested resolution to 3/18 - but should
be obvious resolution.
@@ -1071,7 +1072,6 @@ virStorageFileGetMetadataFromFDInternal(const
char *path,
return ret;
}
-
/**
Spurious whitespace change.
ACK with this squashed in (and any additional trivial cleanups you want
to optionally do based on my comments above):
diff --git i/src/util/virstoragefile.c w/src/util/virstoragefile.c
index b0fe2ae..ff8de43 100644
--- i/src/util/virstoragefile.c
+++ w/src/util/virstoragefile.c
@@ -948,8 +948,12 @@ virStorageFileMetadataNew(const char *path,
if (VIR_STRDUP(ret->path, path) < 0)
goto error;
- if (!(ret->canonPath = canonicalize_file_name(path))) {
- virReportSystemError(errno, _("unable to resolve '%s'"),
path);
+ if (virStorageIsFile(path)) {
+ if (!(ret->canonPath = canonicalize_file_name(path))) {
+ virReportSystemError(errno, _("unable to resolve '%s'"),
path);
+ goto error;
+ }
+ } else if (VIR_STRDUP(ret->canonPath, path) < 0) {
goto error;
}
@@ -1072,6 +1076,7 @@
virStorageFileGetMetadataFromFDInternal(virStorageFileMetadataPtr meta,
return ret;
}
+
/**
* virStorageFileGetMetadataFromFD:
*
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org