storage: Don't store parent directory of an image explicitly

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.

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index f53f99e03bdf50f933492ceeea8a7fdb65c5dd3d..8c0c5d6ac787f8edd8327dcaf89b86747d26bbd9 100644 (file)
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2837,8 +2837,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 */
@@ -2945,19 +2945,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 fb122bbd5c9e66461cf555b21497340e4a74ab7e..6a57327660db93540483c4805432db0c1b405dc3 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1339,9 +1339,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;
 
@@ -1372,8 +1373,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)
@@ -1381,7 +1394,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
             }
         }
         *parent = chain->path;
-        parentDir = chain->relDir;
+        prev = chain;
         chain = chain->backingStore;
         i++;
     }
@@ -1551,7 +1564,6 @@ virStorageSourceClearBackingStore(virStorageSourcePtr def)
         return;
 
     VIR_FREE(def->relPath);
-    VIR_FREE(def->relDir);
     VIR_FREE(def->backingStoreRaw);
 
     /* recursively free backing chain */
@@ -1607,7 +1619,6 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
                                        const char *rel)
 {
     char *dirname = NULL;
-    const char *parentdir = "";
     virStorageSourcePtr ret;
 
     if (VIR_ALLOC(ret) < 0)
@@ -1617,23 +1628,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 */
@@ -1644,12 +1652,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:
@@ -1865,12 +1870,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 {
@@ -1884,17 +1883,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 272a44d31986589d16dfe71085d338dfd98d7c6c..f98a76379a8801a921f199a35dec67c3ca7fc746 100644 (file)
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -250,9 +250,6 @@ struct _virStorageSource {
     /* Relative name by which this image was opened from its parent, or NULL
      * if this image was opened by absolute name */
     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 e494bf376f5e5188a827c9afc7003d21632bb538..fb2837f1de5b0c486100c7ccd0bd686a2dd5e2a2 100644 (file)
--- 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);
@@ -713,7 +704,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,
     };
@@ -730,13 +720,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,
     };
@@ -769,7 +757,6 @@ mymain(void)
         .expBackingStoreRaw = absqcow2,
         .expCapacity = 1024,
         .path = canonwrap,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QCOW2,
     };
@@ -795,7 +782,6 @@ mymain(void)
         .expBackingStoreRaw = absqcow2,
         .expCapacity = 1024,
         .path = canonwrap,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_FILE,
         .format = VIR_STORAGE_FILE_QCOW2,
     };
@@ -843,7 +829,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,
@@ -854,13 +839,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,
     };
@@ -871,7 +854,6 @@ mymain(void)
     /* directory */
     testFileData dir = {
         .path = canondir,
-        .relDir = datadir,
         .type = VIR_STORAGE_TYPE_DIR,
         .format = VIR_STORAGE_FILE_DIR,
     };
@@ -904,7 +886,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,
     };
@@ -912,14 +893,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);