[PATCHv3 28/36] storage: Don't canonicalize paths unnecessarily

[Peter Krempa <pkrempa@xxxxxxxxxx>]

Store backing chain paths as non-cannonical. The cannonicalization step
will be already taken. This will allow to avoid storing unnecessary
amounts of data.
---
 src/util/virstoragefile.c | 33 ++++++---------------------------
 tests/virstoragetest.c    | 10 +++++-----
 2 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 6482689..02b4c73 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1043,25 +1043,17 @@ virStorageFileGetMetadataFromFD(const char *path,
                                 int *backingFormat)

 {
-    virStorageSourcePtr ret = NULL;
-    char *canonPath = NULL;
-
-    if (!(canonPath = canonicalize_file_name(path))) {
-        virReportSystemError(errno, _("unable to resolve '%s'"), path);
-        goto cleanup;
-    }
+    virStorageSourcePtr ret;

-    if (!(ret = virStorageFileMetadataNew(canonPath, format)))
-        goto cleanup;
+    if (!(ret = virStorageFileMetadataNew(path, format)))
+        return NULL;


     if (virStorageFileGetMetadataFromFDInternal(ret, fd, backingFormat) < 0) {
         virStorageSourceFree(ret);
-        ret = NULL;
+        return NULL;
     }

- cleanup:
-    VIR_FREE(canonPath);
     return ret;
 }

@@ -1610,15 +1602,6 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
             virReportOOMError();
             goto error;
         }
-
-        /* XXX we don't currently need to store the canonical path but the
-         * change would break the test suite. Get rid of this later */
-        char *tmp = ret->path;
-        if (!(ret->path = canonicalize_file_name(tmp))) {
-            ret->path = tmp;
-            tmp = NULL;
-        }
-        VIR_FREE(tmp);
     } else {
         ret->type = VIR_STORAGE_TYPE_NETWORK;

@@ -1857,12 +1840,8 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
             goto error;
         }

-        /* XXX we don't currently need to store the canonical path but the
-         * change would break the test suite. Get rid of this later */
-        if (!(ret->path = canonicalize_file_name(path))) {
-            if (VIR_STRDUP(ret->path, path) < 0)
-                goto error;
-        }
+        if (VIR_STRDUP(ret->path, path) < 0)
+            goto error;
     } else {
         ret->type = VIR_STORAGE_TYPE_NETWORK;

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 3c262e2..5ce42d3 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -121,10 +121,8 @@ testStorageFileGetMetadata(const char *path,
         goto error;
     }

-    if (!(ret->path = canonicalize_file_name(path))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "failed to resolve '%s'", path);
+    if (VIR_STRDUP(ret->path, path) < 0)
         goto error;
-    }

     if (virStorageFileGetMetadata(ret, uid, gid, allow_probe) < 0)
         goto error;
@@ -937,7 +935,7 @@ mymain(void)
         .expBackingStoreRaw = "../raw",
         .expCapacity = 1024,
         .pathRel = "../sub/link1",
-        .path = canonqcow2,
+        .path = datadir "/sub/../sub/link1",
         .relDir = datadir "/sub/../sub",
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QCOW2,
@@ -945,11 +943,13 @@ mymain(void)
     testFileData link2 = {
         .expBackingStoreRaw = "../sub/link1",
         .expCapacity = 1024,
-        .path = canonwrap,
+        .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,
-- 
1.9.3

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