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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]