On 02/15/13 21:38, Eric Blake wrote:
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
* 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 | 122 +++++++++++++++++++++-------------------------
1 file changed, 56 insertions(+), 66 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 83b00e2..d741d3d 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -658,22 +658,63 @@ cleanup:
static virStorageFileMetadataPtr
-virStorageFileGetMetadataFromBuf(int format,
- const char *path,
- unsigned char *buf,
- size_t len,
- virStorageFileMetadata *meta)
+virStorageFileGetMetadataInternal(const char *path,
+ int fd,
+ int 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);
- VIR_DEBUG("path=%s format=%d", path, 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);
Preexisting, but: s/header/header of/
+ 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)
+ if (!fileTypeInfo[format].magic)
goto done;
/* Optionally extract capacity from file */
@@ -747,7 +788,11 @@ virStorageFileGetMetadataFromBuf(int format,
done:
ret = meta;
+ meta = NULL;
+
cleanup:
+ virStorageFileFreeMetadata(meta);
+ VIR_FREE(buf);
return ret;
}
Ah, now the change makes sense.
ACK.
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list