[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 3/5] storage: refactor metadata lookup



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]