From 9a39f504209c24d559fc201a327103aa3b5fe091 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Tue, 27 May 2014 15:32:21 +0200 Subject: [PATCH] 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 | 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 f53f99e03..8c0c5d6ac 100644 --- 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 fb122bbd5..6a5732766 100644 --- 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 272a44d31..f98a76379 100644 --- 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 e494bf376..fb2837f1d 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); @@ -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); -- 2.39.5