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.

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",
+    VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d",
-              src->path, NULLSTR(src->relDir), src->format,
+              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;

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
+    if (!(dirname = mdir_name(parent->path))) {
-     * able to use mdir_name on parent->path instead of using parent->relDir */
+        virReportOOMError();
-    if (STRNEQ(parent->relDir, "/"))
-        parentdir = parent->relDir;
-
-    if (virAsprintf(&ret->path, "%s/%s", parentdir, rel) < 0)
         goto error;
+    }
 
-    if (virStorageSourceGetActualType(parent) != VIR_STORAGE_TYPE_NETWORK) {
+    if (STRNEQ(dirname, "/")) {
-        ret->type = VIR_STORAGE_TYPE_FILE;
+        if (virAsprintf(&ret->path, "%s/%s", dirname, rel) < 0)
-
-        /* XXX store the relative directory name for test's sake */
-        if (!(ret->relDir = mdir_name(ret->path))) {
-            virReportOOMError();
             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;
-
+    } else {
-        /* XXX store the relative directory name for test's sake */
+        /* set the type to _FILE, the caller shall update it to the actual type */
-        if (!(ret->relDir = mdir_name(ret->path))) {
+        ret->type = VIR_STORAGE_TYPE_FILE;
-            virReportOOMError();
-            goto error;
-        }
     }
 
  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;

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;

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);