[libvirt] [PATCH 4/5] conf: return backing information separately from metadata

Eric Blake eblake at redhat.com
Thu Apr 10 03:41:37 UTC 2014


A couple pieces of virStorageFileMetadata are used only while
collecting information about the chain, and don't need to
live permanently in the struct.  This patch refactors external
callers to collect the information separately, so that the
next patch can remove the fields.

* src/util/virstoragefile.h (virStorageFileGetMetadataFromBuf):
Alter signature.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal):
Likewise.
(virStorageFileGetMetadataFromFDInternal): Adjust callers.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Likewise.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Likewise.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 src/storage/storage_backend_fs.c      | 34 ++++++++++++++-----------------
 src/storage/storage_backend_gluster.c | 14 ++++++-------
 src/util/virstoragefile.c             | 38 ++++++++++++++++++++---------------
 src/util/virstoragefile.h             |  6 +++++-
 4 files changed, 48 insertions(+), 44 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 5730e3b..be963d4 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -106,7 +106,9 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,

         if (!(meta = virStorageFileGetMetadataFromBuf(target->path,
                                                       header, len,
-                                                      target->format))) {
+                                                      target->format,
+                                                      backingStore,
+                                                      backingStoreFormat))) {
             ret = -1;
             goto error;
         }
@@ -114,25 +116,19 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,

     VIR_FORCE_CLOSE(fd);

-    if (meta && meta->backingStore) {
-        *backingStore = meta->backingStore;
-        meta->backingStore = NULL;
-        if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO &&
-            virStorageIsFile(*backingStore)) {
-            if ((ret = virStorageFileProbeFormat(*backingStore, -1, -1)) < 0) {
-                /* If the backing file is currently unavailable, only log an error,
-                 * but continue. Returning -1 here would disable the whole storage
-                 * pool, making it unavailable for even maintenance. */
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("cannot probe backing volume format: %s"),
-                               *backingStore);
-                ret = -3;
-            } else {
-                *backingStoreFormat = ret;
-                ret = 0;
-            }
+    if (meta && *backingStore &&
+        *backingStoreFormat == VIR_STORAGE_FILE_AUTO &&
+        virStorageIsFile(*backingStore)) {
+        if ((ret = virStorageFileProbeFormat(*backingStore, -1, -1)) < 0) {
+            /* If the backing file is currently unavailable, only log an error,
+             * but continue. Returning -1 here would disable the whole storage
+             * pool, making it unavailable for even maintenance. */
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("cannot probe backing volume format: %s"),
+                           *backingStore);
+            ret = -3;
         } else {
-            *backingStoreFormat = meta->backingStoreFormat;
+            *backingStoreFormat = ret;
             ret = 0;
         }
     } else {
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index 2593c1c..e0a25df 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -298,16 +298,14 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
                                                                len)) < 0)
         goto cleanup;
     if (!(meta = virStorageFileGetMetadataFromBuf(name, header, len,
-                                                  vol->target.format)))
+                                                  vol->target.format,
+                                                  &vol->backingStore.path,
+                                                  &vol->backingStore.format)))
         goto cleanup;

-    if (meta->backingStore) {
-        vol->backingStore.path = meta->backingStore;
-        meta->backingStore = NULL;
-        vol->backingStore.format = meta->backingStoreFormat;
-        if (vol->backingStore.format < 0)
-            vol->backingStore.format = VIR_STORAGE_FILE_RAW;
-    }
+    if (vol->backingStore.path &&
+        vol->backingStore.format < 0)
+        vol->backingStore.format = VIR_STORAGE_FILE_RAW;
     if (meta->capacity)
         vol->target.capacity = meta->capacity;
     if (meta->encryption) {
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index fe8144a..c509a9a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -790,13 +790,16 @@ qcow2GetFeatures(virBitmapPtr *features,
  * information about the file and its backing store.  */
 static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
 ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(7)
+ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(9)
 virStorageFileGetMetadataInternal(const char *path,
                                   const char *canonPath,
                                   const char *directory,
                                   char *buf,
                                   size_t len,
                                   int format,
-                                  virStorageFileMetadataPtr meta)
+                                  virStorageFileMetadataPtr meta,
+                                  char **backingStore,
+                                  int *backingFormat)
 {
     int ret = -1;

@@ -855,10 +858,9 @@ virStorageFileGetMetadataInternal(const char *path,
     }

     if (fileTypeInfo[format].getBackingStore != NULL) {
-        char *backing;
-        int backingFormat;
+        char *backing = NULL;
         int store = fileTypeInfo[format].getBackingStore(&backing,
-                                                         &backingFormat,
+                                                         backingFormat,
                                                          buf, len);
         if (store == BACKING_STORE_INVALID)
             goto done;
@@ -880,23 +882,20 @@ virStorageFileGetMetadataInternal(const char *path,
                     /* the backing file is (currently) unavailable, treat this
                      * file as standalone:
                      * backingStoreRaw is kept to mark broken image chains */
-                    backingFormat = VIR_STORAGE_FILE_NONE;
+                    *backingFormat = VIR_STORAGE_FILE_NONE;
                     VIR_WARN("Backing file '%s' of image '%s' is missing.",
                              meta->backingStoreRaw, path);

                 }
             } else {
-                if (VIR_STRDUP(meta->backingStoreRaw, backing) < 0) {
-                    VIR_FREE(backing);
-                    goto cleanup;
-                }
-                backingFormat = VIR_STORAGE_FILE_RAW;
+                *backingStore = backing;
+                backing = NULL;
+                *backingFormat = VIR_STORAGE_FILE_RAW;
             }
             VIR_FREE(backing);
-            meta->backingStoreFormat = backingFormat;
         } else {
             meta->backingStore = NULL;
-            meta->backingStoreFormat = VIR_STORAGE_FILE_NONE;
+            *backingFormat = VIR_STORAGE_FILE_NONE;
         }
     }

@@ -980,6 +979,8 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)
  * @buf: header bytes from @path
  * @len: length of @buf
  * @format: expected image format
+ * @backing: output malloc'd name of backing image, if any
+ * @backingFormat: format of @backing
  *
  * Extract metadata about the storage volume with the specified
  * image format. If image format is VIR_STORAGE_FILE_AUTO, it
@@ -989,7 +990,7 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)
  * format, since a malicious guest can turn a raw file into any
  * other non-raw format at will.
  *
- * If the returned meta.backingStoreFormat is VIR_STORAGE_FILE_AUTO
+ * If the returned @backingFormat is VIR_STORAGE_FILE_AUTO
  * it indicates the image didn't specify an explicit format for its
  * backing store. Callers are advised against probing for the
  * backing store format in this case.
@@ -1000,7 +1001,9 @@ virStorageFileMetadataPtr
 virStorageFileGetMetadataFromBuf(const char *path,
                                  char *buf,
                                  size_t len,
-                                 int format)
+                                 int format,
+                                 char **backing,
+                                 int *backingFormat)
 {
     virStorageFileMetadataPtr ret = NULL;
     char *canonPath;
@@ -1013,7 +1016,8 @@ virStorageFileGetMetadataFromBuf(const char *path,
         goto cleanup;

     if (virStorageFileGetMetadataInternal(path, canonPath, ".", buf, len,
-                                          format, ret) < 0) {
+                                          format, ret, backing,
+                                          backingFormat) < 0) {
         virStorageFileFreeMetadata(ret);
         ret = NULL;
     }
@@ -1068,7 +1072,9 @@ virStorageFileGetMetadataFromFDInternal(const char *path,
     }

     ret = virStorageFileGetMetadataInternal(path, canonPath, directory,
-                                            buf, len, format, meta);
+                                            buf, len, format, meta,
+                                            &meta->backingStoreRaw,
+                                            &meta->backingStoreFormat);

     if (ret == 0) {
         if (S_ISREG(sb.st_mode))
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 4da160b..edfe9eb 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -295,7 +295,11 @@ virStorageFileMetadataPtr virStorageFileGetMetadataFromFD(const char *path,
 virStorageFileMetadataPtr virStorageFileGetMetadataFromBuf(const char *path,
                                                            char *buf,
                                                            size_t len,
-                                                           int format);
+                                                           int format,
+                                                           char **backing,
+                                                           int *backingFormat)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5)
+    ATTRIBUTE_NONNULL(6);
 int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain,
                                  char **broken_file);

-- 
1.9.0




More information about the libvir-list mailing list