[libvirt] [PATCHv3 29/36] storage: Don't store parent directory of an image explicitly

Peter Krempa pkrempa at redhat.com
Fri May 30 08:37:51 UTC 2014


The parent directory doesn't necessarily need to be stored after we
don't mangle the path stored in the image. Remove it and tweak the code
to avoid using it.
---
 src/storage/storage_driver.c | 11 ++-----
 src/util/virstoragefile.c    | 68 ++++++++++++++++++--------------------------
 src/util/virstoragefile.h    |  3 --
 tests/virstoragetest.c       | 21 --------------
 4 files changed, 30 insertions(+), 73 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9ce3b62..1c8ad1e 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3082,8 +3082,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
     virStorageSourcePtr backingStore = NULL;
     int backingFormat;

-    VIR_DEBUG("path=%s dir=%s format=%d uid=%d gid=%d probe=%d",
-              src->path, NULLSTR(src->relDir), src->format,
+    VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d",
+              src->path, src->format,
               (int)uid, (int)gid, allow_probe);

     /* exit if we can't load information about the current image */
@@ -3189,19 +3189,12 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
     if (!(cycle = virHashCreate(5, NULL)))
         return -1;

-    if (!src->relDir &&
-        !(src->relDir = mdir_name(src->path))) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
     if (src->format <= VIR_STORAGE_FILE_NONE)
         src->format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;

     ret = virStorageFileGetMetadataRecurse(src, uid, gid,
                                            allow_probe, cycle);

- cleanup:
     VIR_FREE(canonPath);
     virHashFree(cycle);
     return ret;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 02b4c73..2a45e58 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1337,9 +1337,10 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
                           unsigned int idx,
                           const char **parent)
 {
+    virStorageSourcePtr prev = NULL;
     const char *start = chain->path;
     const char *tmp;
-    const char *parentDir = ".";
+    char *parentDir = NULL;
     bool nameIsFile = virStorageIsFile(name);
     size_t i;

@@ -1369,8 +1370,20 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
                 break;
             if (nameIsFile && (chain->type == VIR_STORAGE_TYPE_FILE ||
                                chain->type == VIR_STORAGE_TYPE_BLOCK)) {
+                if (prev) {
+                    if (!(parentDir = mdir_name(prev->path))) {
+                        virReportOOMError();
+                        goto error;
+                    }
+                } else {
+                    if (VIR_STRDUP(parentDir, ".") < 0)
+                        goto error;
+                }
+
                 int result = virFileRelLinkPointsTo(parentDir, name,
                                                     chain->path);
+
+                VIR_FREE(parentDir);
                 if (result < 0)
                     goto error;
                 if (result > 0)
@@ -1378,7 +1391,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
             }
         }
         *parent = chain->path;
-        parentDir = chain->relDir;
+        prev = chain;
         chain = chain->backingStore;
         i++;
     }
@@ -1517,7 +1530,6 @@ virStorageSourceClearBackingStore(virStorageSourcePtr def)
         return;

     VIR_FREE(def->relPath);
-    VIR_FREE(def->relDir);
     VIR_FREE(def->backingStoreRaw);

     /* recursively free backing chain */
@@ -1576,7 +1588,6 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
                                        const char *rel)
 {
     char *dirname = NULL;
-    const char *parentdir = "";
     virStorageSourcePtr ret;

     if (VIR_ALLOC(ret) < 0)
@@ -1586,23 +1597,20 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
     if (VIR_STRDUP(ret->relPath, parent->backingStoreRaw) < 0)
         goto error;

-    /* XXX Once we get rid of the need to use canonical names in path, we will be
-     * able to use mdir_name on parent->path instead of using parent->relDir */
-    if (STRNEQ(parent->relDir, "/"))
-        parentdir = parent->relDir;
-
-    if (virAsprintf(&ret->path, "%s/%s", parentdir, rel) < 0)
+    if (!(dirname = mdir_name(parent->path))) {
+        virReportOOMError();
         goto error;
+    }

-    if (virStorageSourceGetActualType(parent) != VIR_STORAGE_TYPE_NETWORK) {
-        ret->type = VIR_STORAGE_TYPE_FILE;
-
-        /* XXX store the relative directory name for test's sake */
-        if (!(ret->relDir = mdir_name(ret->path))) {
-            virReportOOMError();
+    if (STRNEQ(dirname, "/")) {
+        if (virAsprintf(&ret->path, "%s/%s", dirname, rel) < 0)
             goto error;
-        }
     } else {
+        if (virAsprintf(&ret->path, "/%s", rel) < 0)
+            goto error;
+    }
+
+    if (virStorageSourceGetActualType(parent) == VIR_STORAGE_TYPE_NETWORK) {
         ret->type = VIR_STORAGE_TYPE_NETWORK;

         /* copy the host network part */
@@ -1613,12 +1621,9 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,

         if (VIR_STRDUP(ret->volume, parent->volume) < 0)
             goto error;
-
-        /* XXX store the relative directory name for test's sake */
-        if (!(ret->relDir = mdir_name(ret->path))) {
-            virReportOOMError();
-            goto error;
-        }
+    } else {
+        /* set the type to _FILE, the caller shall update it to the actual type */
+        ret->type = VIR_STORAGE_TYPE_FILE;
     }

  cleanup:
@@ -1834,12 +1839,6 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
     if (virStorageIsFile(path)) {
         ret->type = VIR_STORAGE_TYPE_FILE;

-        /* XXX store the relative directory name for test's sake */
-        if (!(ret->relDir = mdir_name(path))) {
-            virReportOOMError();
-            goto error;
-        }
-
         if (VIR_STRDUP(ret->path, path) < 0)
             goto error;
     } else {
@@ -1853,17 +1852,6 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
             if (virStorageSourceParseBackingColon(ret, path) < 0)
                 goto error;
         }
-
-        /* XXX fill relative path so that relative names work with network storage too */
-        if (ret->path) {
-            if (!(ret->relDir = mdir_name(ret->path))) {
-                virReportOOMError();
-                goto error;
-            }
-        } else {
-            if (VIR_STRDUP(ret->relDir, "") < 0)
-                goto error;
-        }
     }

     return ret;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 37f4e66..a83e168 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -250,9 +250,6 @@ struct _virStorageSource {
     /* Relative path of the backing image from the parent NULL if
      * backed by absolute path */
     char *relPath;
-    /* Directory to start from if backingStoreRaw is a relative file
-     * name.  */
-    char *relDir;
     /* Name of the child backing store recorded in metadata of the
      * current file.  */
     char *backingStoreRaw;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 5ce42d3..a09648c 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -116,11 +116,6 @@ testStorageFileGetMetadata(const char *path,
         }
     }

-    if (!(ret->relDir = mdir_name(path))) {
-        virReportOOMError();
-        goto error;
-    }
-
     if (VIR_STRDUP(ret->path, path) < 0)
         goto error;

@@ -282,7 +277,6 @@ struct _testFileData
     bool expEncrypted;
     const char *pathRel;
     const char *path;
-    const char *relDir;
     int type;
     int format;
 };
@@ -311,7 +305,6 @@ static const char testStorageChainFormat[] =
     "capacity: %lld\n"
     "encryption: %d\n"
     "relPath:%s\n"
-    "relDir:%s\n"
     "type:%d\n"
     "format:%d\n";

@@ -375,7 +368,6 @@ testStorageChain(const void *args)
                         data->files[i]->expCapacity,
                         data->files[i]->expEncrypted,
                         NULLSTR(data->files[i]->pathRel),
-                        NULLSTR(data->files[i]->relDir),
                         data->files[i]->type,
                         data->files[i]->format) < 0 ||
             virAsprintf(&actual,
@@ -385,7 +377,6 @@ testStorageChain(const void *args)
                         elt->capacity,
                         !!elt->encryption,
                         NULLSTR(elt->relPath),
-                        NULLSTR(elt->relDir),
                         elt->type,
                         elt->format) < 0) {
             VIR_FREE(expect);
@@ -745,7 +736,6 @@ mymain(void)
     /* Raw image, whether with right format or no specified format */
     testFileData raw = {
         .path = canonraw,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_RAW,
     };
@@ -762,13 +752,11 @@ mymain(void)
         .expBackingStoreRaw = "raw",
         .expCapacity = 1024,
         .path = canonqcow2,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QCOW2,
     };
     testFileData qcow2_as_raw = {
         .path = canonqcow2,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_RAW,
     };
@@ -801,7 +789,6 @@ mymain(void)
         .expBackingStoreRaw = absqcow2,
         .expCapacity = 1024,
         .path = canonwrap,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QCOW2,
     };
@@ -827,7 +814,6 @@ mymain(void)
         .expBackingStoreRaw = absqcow2,
         .expCapacity = 1024,
         .path = canonwrap,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QCOW2,
     };
@@ -875,7 +861,6 @@ mymain(void)
         .path = "blah",
         .type = VIR_STORAGE_TYPE_NETWORK,
         .format = VIR_STORAGE_FILE_RAW,
-        .relDir = ".",
     };
     TEST_CHAIN(11, absqcow2, VIR_STORAGE_FILE_QCOW2,
                (&qcow2, &nbd), EXP_PASS,
@@ -886,13 +871,11 @@ mymain(void)
         .expBackingStoreRaw = absraw,
         .expCapacity = 1024,
         .path = canonqed,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QED,
     };
     testFileData qed_as_raw = {
         .path = canonqed,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_RAW,
     };
@@ -903,7 +886,6 @@ mymain(void)
     /* directory */
     testFileData dir = {
         .path = canondir,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_DIR,
         .format = VIR_STORAGE_FILE_DIR,
     };
@@ -936,7 +918,6 @@ mymain(void)
         .expCapacity = 1024,
         .pathRel = "../sub/link1",
         .path = datadir "/sub/../sub/link1",
-        .relDir = datadir "/sub/../sub",
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QCOW2,
     };
@@ -944,14 +925,12 @@ mymain(void)
         .expBackingStoreRaw = "../sub/link1",
         .expCapacity = 1024,
         .path = abslink2,
-        .relDir = datadir "/sub",
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QCOW2,
     };

     raw.path = datadir "/sub/../sub/../raw";
     raw.pathRel = "../raw";
-    raw.relDir = datadir "/sub/../sub/..";
     TEST_CHAIN(15, abslink2, VIR_STORAGE_FILE_QCOW2,
                (&link2, &link1, &raw), EXP_PASS,
                (&link2, &link1, &raw), ALLOW_PROBE | EXP_PASS);
-- 
1.9.3




More information about the libvir-list mailing list